fix(diagnostics): enforce current-editor snapshots and stale results.#95
Merged
knightedcodemonkey merged 2 commits intonextfrom Apr 24, 2026
Merged
fix(diagnostics): enforce current-editor snapshots and stale results.#95knightedcodemonkey merged 2 commits intonextfrom
knightedcodemonkey merged 2 commits intonextfrom
Conversation
Owner
knightedcodemonkey
commented
Apr 24, 2026
- fixes Diagnostics race conditions and stale results in multi-tab editor, plus drawer UX no longer matches tab language #93
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses diagnostics race conditions (#93) by making lint/typecheck runs commit results only when they match the current editor/tab snapshot, and updates the diagnostics drawer to better reflect multi-tab workflows.
Changes:
- Add run identity + source version guards to prevent stale typecheck and lint completions from overwriting newer state.
- Capture current editor/tab snapshots when triggering diagnostics to ensure runs always evaluate the intended source/path.
- Update diagnostics drawer layout/labels for styles vs component tabs and extend Playwright coverage for stale-result scenarios.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/modules/diagnostics/type-diagnostics.js | Adds sourceVersion + workspace tab snapshot overrides to ignore late typecheck results and stabilize auto-recheck timing. |
| src/modules/diagnostics/lint-diagnostics.js | Adds runContext + isRunContextCurrent commit guards and exposes targeted cancel methods. |
| src/modules/app-core/workspace-tab-selection-controller.js | Adds onActiveWorkspaceTabChange callback to react to tab selections. |
| src/modules/app-core/workspace-controllers-setup.js | Wires onActiveWorkspaceTabChange into tab selection controller setup. |
| src/modules/app-core/diagnostics-tab-state-helpers.js | New helper for drawer layout sync, tab-switch clearing, and lint target identity resolution. |
| src/modules/app-core/diagnostics-flow-controller.js | Adds lint run context identity/versioning and cancels lint runs on staleness; adjusts drawer/status behavior. |
| src/modules/app-core/app-composition-options.js | Plumbs lint target getters into runtime core options. |
| src/modules/app-core/app-bindings-startup.js | Ensures diagnostics run off persisted active-tab snapshots; adjusts reset behavior and initial drawer layout sync. |
| src/app.js | Wires new diagnostics helpers, stores flow controller reference, and plumbs snapshot + tab-kind helpers. |
| playwright/diagnostics.spec.ts | Updates reset button expectations and adds regression tests for stale results + tab-driven drawer layout. |
Comments suppressed due to low confidence (2)
src/modules/app-core/diagnostics-flow-controller.js:196
scheduledComponentLintRecheck/scheduledStylesLintRecheckand their clear helpers remain, but the PR removed all code paths that ever assign these timers. This leaves dead state + branches that are now misleading. Either remove the unused timer variables/helpers, or reintroduce the scheduling logic if auto re-check is still intended for lint.
let scheduledComponentLintRecheck = null
let scheduledStylesLintRecheck = null
let componentLintPending = false
let stylesLintPending = false
let componentLintSourceVersion = 0
let stylesLintSourceVersion = 0
const normalizeLintTargetIdentity = target => {
const tabId = typeof target?.tabId === 'string' ? target.tabId.trim() : ''
const path = typeof target?.path === 'string' ? target.path.trim() : ''
const language =
typeof target?.language === 'string' ? target.language.trim().toLowerCase() : ''
return {
tabId,
path,
language,
key: `${tabId}|${path}|${language}`,
}
}
const getCurrentLintTargetIdentity = scope => {
const resolveTarget =
scope === 'styles' ? getStylesLintTarget : getComponentLintTarget
return normalizeLintTargetIdentity(resolveTarget())
}
const createLintRunContext = scope => {
const target = getCurrentLintTargetIdentity(scope)
const sourceVersion =
scope === 'styles' ? stylesLintSourceVersion : componentLintSourceVersion
return {
scope,
sourceVersion,
targetKey: target.key,
}
}
const isLintRunContextCurrent = (scope, runContext) => {
if (!runContext || runContext.scope !== scope) {
return false
}
const currentSourceVersion =
scope === 'styles' ? stylesLintSourceVersion : componentLintSourceVersion
if (runContext.sourceVersion !== currentSourceVersion) {
return false
}
const currentTarget = getCurrentLintTargetIdentity(scope)
return runContext.targetKey === currentTarget.key
}
const setTypecheckButtonLoading = isLoading => {
const runtimeTypecheckButton = document.getElementById('typecheck-button')
if (!(runtimeTypecheckButton instanceof HTMLButtonElement)) {
return
}
runtimeTypecheckButton.classList.toggle('render-button--loading', isLoading)
runtimeTypecheckButton.setAttribute('aria-busy', isLoading ? 'true' : 'false')
runtimeTypecheckButton.disabled = isLoading
}
const setLintButtonLoading = ({ button, isLoading }) => {
if (!(button instanceof HTMLButtonElement)) {
return
}
button.classList.toggle('render-button--loading', isLoading)
button.setAttribute('aria-busy', isLoading ? 'true' : 'false')
button.disabled = isLoading
}
const typeDiagnostics = createTypeDiagnosticsController({
cdnImports,
importFromCdnWithFallback,
getTypeScriptLibUrls,
getTypePackageFileUrls,
getJsxSource,
getTypecheckSourcePath,
getWorkspaceTabs,
getRenderMode,
setTypecheckButtonLoading,
setTypeDiagnosticsDetails,
setTypeDiagnosticsPending,
setStatus,
setRenderedStatus: () => {
if (typeDiagnostics.getLastTypeErrorCount() > 0) {
setStatus(
`Rendered (Type errors: ${typeDiagnostics.getLastTypeErrorCount()})`,
'error',
)
return
}
if (statusNode.textContent.startsWith('Rendered (Type errors:')) {
setStatus('Rendered', 'neutral')
}
},
isRenderedStatus: () =>
statusNode.textContent === 'Rendered' ||
statusNode.textContent.startsWith('Rendered (Type errors:'),
isRenderedTypeErrorStatus: () =>
statusNode.textContent.startsWith('Rendered (Type errors:'),
incrementTypeDiagnosticsRuns,
decrementTypeDiagnosticsRuns,
getActiveTypeDiagnosticsRuns,
onIssuesDetected: ({ issueCount }) => {
if (issueCount > 0) {
setDiagnosticsDrawerOpen(true)
}
},
})
const setRenderedStatus = () => {
if (typeDiagnostics.getLastTypeErrorCount() > 0) {
setStatus(
`Rendered (Type errors: ${typeDiagnostics.getLastTypeErrorCount()})`,
'error',
)
return
}
if (statusNode.textContent.startsWith('Rendered (Type errors:')) {
setStatus('Rendered', 'neutral')
}
}
const lintDiagnostics = createLintDiagnosticsController({
cdnImports,
importFromCdnWithFallback,
getComponentSource: getJsxSource,
getStylesSource: getCssSource,
getStyleMode,
setComponentDiagnostics: setStyleDiagnosticsDetails,
setStyleDiagnostics: setStyleDiagnosticsDetails,
setStatus,
onIssuesDetected: ({ issueCount }) => {
if (issueCount > 0) {
setDiagnosticsDrawerOpen(true)
}
},
})
const clearComponentLintRecheckTimer = () => {
if (scheduledComponentLintRecheck) {
clearTimeout(scheduledComponentLintRecheck)
scheduledComponentLintRecheck = null
}
}
const clearStylesLintRecheckTimer = () => {
if (scheduledStylesLintRecheck) {
clearTimeout(scheduledStylesLintRecheck)
scheduledStylesLintRecheck = null
}
}
src/modules/app-core/diagnostics-flow-controller.js:327
- The updated lint staleness behavior no longer schedules an automatic re-check when unresolved lint issues exist; it always marks lint as stale and requires a manual rerun. This contradicts the issue/#93 requirements to preserve the auto re-check-after-edit behavior when unresolved issues are present. Consider restoring a per-scope
hasUnresolvedLintIssues/lastLintIssueCount+ debounce scheduling (with run identity + sourceVersion/target guards), and only switch to manual rerun when there were no unresolved issues.
const markComponentLintDiagnosticsStale = () => {
componentLintSourceVersion += 1
clearComponentLintRecheckTimer()
activeComponentLintAbortController?.abort()
activeComponentLintAbortController = null
lintDiagnostics.cancelComponent()
setLintButtonLoading({ button: lintComponentButton, isLoading: false })
componentLintPending = false
syncLintPendingState()
setStyleDiagnosticsDetails({
headline: 'Source changed. Click Lint to run diagnostics.',
level: 'muted',
})
if (statusNode.textContent.startsWith('Rendered (Lint issues:')) {
setStatus('Rendered', 'neutral')
}
}
const markStylesLintDiagnosticsStale = () => {
stylesLintSourceVersion += 1
clearStylesLintRecheckTimer()
activeStylesLintAbortController?.abort()
activeStylesLintAbortController = null
lintDiagnostics.cancelStyles()
setLintButtonLoading({ button: lintStylesButton, isLoading: false })
stylesLintPending = false
syncLintPendingState()
setStyleDiagnosticsDetails({
headline: 'Source changed. Click Lint to run diagnostics.',
level: 'muted',
})
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.